docs(modules): operator guide for the module substrate (HT-71) - #81
Conversation
📝 WalkthroughWalkthroughAdds documentation for the module substrate v1 HTTP surface, including shared API conventions, assistant draft and approval workflows, and webhook registration, delivery, verification, retry, and health behavior. ChangesModule substrate documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/modules/assistants-and-drafts.md`:
- Around line 8-13: Update the Assistant description in the introductory
paragraph to avoid claiming every write is a draft: state that every
customer-facing reply is a draft, while explicitly allowing internal notes
posted through the conversations notes endpoint. Preserve the existing approval
requirement for customer-facing messages.
- Around line 3-6: Remove the temporary pre-merge status note at the top of the
Assistants and Drafts API guide, including references to branch
feat/ht-70-drafts-approval and PR `#80`; leave the documentation with stable
introductory content suitable for main.
In `@docs/modules/README.md`:
- Around line 3-6: Update the AI actor example in the module substrate
description to use the fixed term “Assistant,” such as “a draft-writing
Assistant,” while preserving the surrounding examples and meaning.
In `@docs/modules/webhooks.md`:
- Around line 257-258: Update the “2xx acks, anything else retries”
documentation row to distinguish retryable HTTP failures, timeouts, and
connection errors from SSRF refusals. State that an SsrfRefusedError is
non-retryable and is immediately dead-lettered on the first delivery attempt,
while preserving the existing redirect behavior.
- Around line 193-213: Update the signature validation in the webhook
verification flow around the signature field and Buffer.from calls: require
signature to be exactly the expected 64 hexadecimal characters before decoding,
rejecting any non-hex or trailing data as a malformed signature. Preserve the
existing timingSafeEqual comparison for validly formatted signatures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 99f13ad7-a3e3-4e11-80f8-7966ce451d39
📒 Files selected for processing (3)
docs/modules/README.mddocs/modules/assistants-and-drafts.mddocs/modules/webhooks.md
Adds docs/modules/{README,webhooks,assistants-and-drafts}.md, written and
verified against the shipped code rather than the spec alone: curl examples
cross-checked against src/api/router.ts's actual patterns and validation
(src/api/webhooks.ts, src/store/webhook-endpoints.ts), and the signature
sample verified byte-for-byte against both the engine's signer
(src/webhooks/delivery.ts) and the reference module's verifier
(module-draft-assistant/src/verify.ts) via a throwaway script (exit 0).
Assistants/drafts sections document PR #80 (HT-70), not yet on main —
flagged at the top of assistants-and-drafts.md; merge this after #80.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- assistants-and-drafts.md: drop the pre-merge branch/PR-#80 status note now that #80 is merged and deployed; content documents merged main. - assistants-and-drafts.md: stop claiming every Assistant write is a draft — notes (POST .../notes) are direct writes with no draft/ approval step, only customer-facing replies are draft-gated. Verified against handlePostNote (src/api/conversations.ts on merged main). - README.md: "draft-writing Assistant", not "draft-writing AI" — vocabulary rule (Assistants, never generic "AI"). - webhooks.md: the signature-verification sample now requires an exact 64-hex-char match before Buffer.from(..., 'hex') decoding — that call otherwise silently truncates at the first non-hex character instead of rejecting trailing garbage after a valid-length prefix. Re-verified with an extended throwaway script (adds the trailing-garbage negative case to the existing engine-signer/reference-module cross-check); exit 0. - webhooks.md: split the delivery-guarantees table's "anything else retries" row — an SsrfRefusedError dead-letters immediately on the first attempt and is never retried, unlike an ordinary HTTP failure/ timeout/connection error. Verified against src/webhooks/delivery.ts's catch branch on merged main. Rebased onto origin/main (PR #80 merged as 475b60e) so both the docs and the verification below reflect merged main, not the pre-merge branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
0f05130 to
f795571
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/modules/README.md`:
- Line 31: Update the “Typed events” description in the module README to remove
the blanket claim that all eight domain events are recorded in the same
transaction as their triggering changes. State that transactional recording or
event timing varies by event type, while preserving the existing event examples
and webhook link.
In `@docs/modules/webhooks.md`:
- Around line 122-126: Update the webhook payload description near the “Events
are thin by design” section to remove the claim that payloads are free of PII by
construction. State narrowly that payloads exclude message content and direct
contact fields, while retaining the guidance to fetch full content through the
read API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2f4deb5b-1bc4-4de5-a7aa-8d338f53d95f
📒 Files selected for processing (3)
docs/modules/README.mddocs/modules/assistants-and-drafts.mddocs/modules/webhooks.md
|
|
||
| | Surface | What it does | Guide | | ||
| |---|---|---| | ||
| | **Typed events** | The engine records eight kinds of domain event (a new conversation, inbound mail, a status change, a resolved draft, …) reliably, in the same transaction as the change they describe. | [webhooks.md](./webhooks.md) | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Remove the blanket same-transaction claim for all events.
conversation.reply_sent is emitted only after outbound delivery is confirmed as sent, so it cannot be recorded in the same transaction as the original conversation change. Describe transactional recording only where it applies, or state that event timing varies by event type.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/modules/README.md` at line 31, Update the “Typed events” description in
the module README to remove the blanket claim that all eight domain events are
recorded in the same transaction as their triggering changes. State that
transactional recording or event timing varies by event type, while preserving
the existing event examples and webhook link.
| **Events are thin by design.** `data` carries only identifiers and small | ||
| typed facts — never a message body, subject line, or address. Fetch full | ||
| content through the read API (`GET /api/v1/conversations/{id}`) with your | ||
| own credentials once an event tells you something changed. This keeps every | ||
| webhook payload free of customer content and PII by construction. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not claim webhook payloads are PII-free by construction.
The documented payload still carries linkable identifiers such as conversationId, threadId, assigneeAgentId, and assistantId. Replace this with a narrower claim that payloads exclude message content and direct contact fields, unless the implementation explicitly guarantees these identifiers are non-personal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/modules/webhooks.md` around lines 122 - 126, Update the webhook payload
description near the “Events are thin by design” section to remove the claim
that payloads are free of PII by construction. State narrowly that payloads
exclude message content and direct contact fields, while retaining the guidance
to fetch full content through the read API.
The doc was last updated 2026-07-12, when the engine was built but not yet live and the UI/deploy were still ahead. Bring it current with everything shipped since: - Live deployment (HT-43): engine at desk.resonantiq.app, Agent Inbox UI at inbox.resonantiq.app, on Supabase + a real Gmail mailbox; live-only Gmail threading bugs found and fixed (HT-49/50). - Agent Inbox UI live (HT-23) and per-Agent identity/login/auth (HT-51, HT-54). - Inbound observability (HT-44, migration 19). - Module substrate era: open-core catalog (HT-66) and substrate v1 spec (HT-67) under specs/modules/; implementation waves 1-3 (HT-68/69/70, migrations 020-023 — actor model, events/webhooks, assistant auth + drafts + approval); first module scaffolded born-proprietary (HT-73). The HT-71 operator guide (docs/modules/, PR #81) is still open, so it is listed under Next rather than Done. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…sics (#91) PR #83 landed already-stale: main moved by nine PRs while it was in review, and it shipped one claim that contradicts the charter. - HT-71 operator guide was listed as an open PR under Next. #81 merged at 17:48, ~2h before #83 merged at 19:39, and docs/modules/ has been on main since. Moved to Done with its precision follow-up (#84). - Marketplace was listed under 'Not yet / deferred'. CHARTER §3/§4/§5 were amended the same day (HT-79, #86) to make it a launch-day component of Phase 3 — built now, proven as the dogfood install path. Removed from deferred; marketplace v1 spec (#87, draft) now leads Next. - Added inbox basics (HT-76/77/78, #90, migrations 24-25): saved replies & macros, snooze, send & close — shipped engine features with no STATUS line. - Added catalog reclassification (HT-75, #82): KB and end-user portal are paid, 71-module gap audit closed, open-core line restated. Passkeys stay core, reconciled in #85. - Added passkey login spec (HT-75, specs/auth/passkeys.md, draft.3) to Next. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What's covered
New
docs/modules/— an operator/module-author guide to Helpthread's module substrate, split into:eventIddedupe, no ordering, thin payloads, SSRF posture), auto-disable behavior and health visibility.Idempotency-Key = eventIdpattern), and the full Agent approval flow (list/approve-with-edits/discard).Written for a stranger-operator — no Resonant-IQ-specific values anywhere (placeholder
your-helpdesk.example.com/your-module.example.comthroughout).What was verified-runnable
src/api/router.ts,src/api/webhooks.ts,src/api/index.ts,src/store/webhook-endpoints.ts(on mergedmain), and againstsrc/api/assistants.ts,src/api/drafts.ts,src/api/assistant-auth.ts,src/auth/assistant-token.tson thefeat/ht-70-drafts-approvalbranch (PR feat(engine): module substrate wave 3 — assistant auth, drafts API, draft-approval (HT-70) #80). Response shapes (field names, status codes, error codes) read directly from the handlers, not inferred from the spec.signWebhookPayload(src/webhooks/delivery.ts), (2) verifies it with the exact function that ended up in the doc, and (3) cross-checks the same signed payload against the independent verifier in the reference module (module-draft-assistant/src/verify.ts) — plus negative controls (wrong secret, tampered body, stale timestamp) all correctly rejected on all three. Script exited0.npm run typecheck— exit 0 (no source touched; docs-only change, run to prove no accidental damage).npm run lint(biome) —Checked 240 files ... No fixes applied— exit 0.Note on scope
The assistants-and-drafts.md sections document functionality that lands on
mainvia PR #80 (HT-70) — flagged explicitly at the top of that file, with the branch path noted as the as-verified source. This PR should merge after #80.Test plan
docs/modules/README.md,webhooks.md,assistants-and-drafts.mdfor accuracy against shipped codemain🤖 Generated with Claude Code
Summary by CodeRabbit
/api/v1surface details, and shared response/error behavior.